@llvm/pr-subscribers-clang

@llvm/pr-subscribers-lldb

Author: Matheus Izvekov (mizvekov)

diff --git a/lldb/DELETE.ME b/lldb/DELETE.ME
new file mode 100644
index 0000000000000..e69de29bb2d1d

✅ With the latest revision this PR passed the Python code formatter.

Hmmmm yea I don't think we want to be losing the scope qualifiers for the typenames. What does LLDB have to do to retain those?

The answer to that depends on what LLDB wants and can do here.

The status quo is, the types produced in the places changed here are not valid as-written types, as they lacked an ElaboratedType. Clang will currently print types lacking an ElaboratedType as if they were fully qualified, with a synthesized name qualifier.

This is used to distinguish canonical from non-canonical types, and the motivation is that we find it more appropriate to print canonical types as fully qualified when they appear in diagnostics and such.

Now with this patch series, we retain the ability to distinguish canonical RecordTypes and EnumTypes, and still print those as fully qualified. But a mis-feature of the status quo is removed: the same would work for TypedefTypes, if they lacked an ElaboratedType on top, then we would fully qualify them when printing.

But this was not an intentional feature, and it lacks motivation, as TypedefTypes never appear in canonical types.
This patch does not create an AST representation for such a TypedefType, beyond what you can do synthesizing a name qualifier yourself.

Now, does LLDB want to keep fully qualified names here, or does it want these to be printed as-written?

Changing this back to the status quo is more straight-forward, we just need to synthesize fully qualified types.

This is taking me a while because it's currently very difficulty for me to get a functional LLDB workflow on MacOS, so
I am currently relying only on the CI job to test and debug this.

To change this to as-written, we would need to figure out if we have a source for this information first, if we store and can retrieve those from all of the Debug info formats we support.

@Michael137 this is ready for another look.

@mizvekov do you have a macOS machine handy to test this on? Or do you want me to try?

@mizvekov do you have a macOS machine handy to test this on? Or do you want me to try?

I don't think there is anything macOS specific about this change.

My main development machine is macOS, and I always struggled with testing LLDB on it.

But I finally setup a Linux VM on it, and all that went fine without a single hitch. Besides, our CI is Linux based, and that passes as well.

@mizvekov do you have a macOS machine handy to test this on? Or do you want me to try?

I don't think there is anything macOS specific about this change.

My main development machine is macOS, and I always struggled with testing LLDB on it.

But I finally setup a Linux VM on it, and all that went fine without a single hitch. Besides, our CI is Linux based, and that passes as well.

There are tests that only run on macOS. I just want to avoid a situation where we have to revert the entire patch series because of some macOS post-merge test failures. Ideally we'd catch these pre-merge on Github, but that's currently not set up unfortunately.

Since you don't have the setup, let me run the tests. I'll report back tomorrow

@mizvekov do you have a macOS machine handy to test this on? Or do you want me to try?

I don't think there is anything macOS specific about this change.

My main development machine is macOS, and I always struggled with testing LLDB on it.

But I finally setup a Linux VM on it, and all that went fine without a single hitch. Besides, our CI is Linux based, and that passes as well.

There are tests that only run on macOS. I just want to avoid a situation where we have to revert the entire patch series because of some macOS post-merge test failures. Ideally we'd catch these pre-merge on Github, but that's currently not set up unfortunately.

Since you don't have the setup, let me run the tests. I'll report back tomorrow

Ah okay. Any easy way for me to tell which are these tests?

I can run some of the tests locally, but not all of them passes, but that's the case for main branch llvm as well.

Hmmm getting this error when building the patch series locally

Undefined symbols for architecture arm64:
  "clang::NestedNameSpecifierLoc::getTypeLoc() const", referenced from:
      clang::RecursiveASTVisitor<(anonymous namespace)::RetainPtrCtorAdoptChecker::checkASTDecl(clang::TranslationUnitDecl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const::LocalVisitor>::TraverseNestedNameSpecifierLoc(clang::NestedNameSpecifierLoc) in libclangStaticAnalyzerCheckers.a[132](RetainPtrCtorAdoptChecker.cpp.o)

Any ideas?

@mizvekov do you have a macOS machine handy to test this on? Or do you want me to try?

I don't think there is anything macOS specific about this change.

My main development machine is macOS, and I always struggled with testing LLDB on it.

But I finally setup a Linux VM on it, and all that went fine without a single hitch. Besides, our CI is Linux based, and that passes as well.

There are tests that only run on macOS. I just want to avoid a situation where we have to revert the entire patch series because of some macOS post-merge test failures. Ideally we'd catch these pre-merge on Github, but that's currently not set up unfortunately.
Since you don't have the setup, let me run the tests. I'll report back tomorrow

Ah okay. Any easy way for me to tell which are these tests?

Hmm not that I'm aware of

Hmmm getting this error when building the patch series locally

Undefined symbols for architecture arm64:
  "clang::NestedNameSpecifierLoc::getTypeLoc() const", referenced from:
      clang::RecursiveASTVisitor<(anonymous namespace)::RetainPtrCtorAdoptChecker::checkASTDecl(clang::TranslationUnitDecl const*, clang::ento::AnalysisManager&, clang::ento::BugReporter&) const::LocalVisitor>::TraverseNestedNameSpecifierLoc(clang::NestedNameSpecifierLoc) in libclangStaticAnalyzerCheckers.a[132](RetainPtrCtorAdoptChecker.cpp.o)

Any ideas?

That's probably missing a #include "clang/AST/NestedNameSpecifier.h" somewhere, where it was previously relying on that being indirectly included.

We do include it from RecursiveASTVisitor.h though.

Is this is building LLVM from a clean build directory?

That builds cleanly on a MacOS machine and the tests for that checker pass.

If you can still reproduce that even from a clean build, please share repro steps.

I would approve if I could. 👍

Clean build succeeded.

These tests fail on macOS:

********************
********************
Unresolved Tests (3):
  lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py
  lldb-api :: commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
  lldb-api :: commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

Clean build succeeded.

These tests fail on macOS:

********************
********************
Unresolved Tests (3):
  lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py
  lldb-api :: commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
  lldb-api :: commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

Thanks, I'll see if I can reproduce locally. Mind sharing the logs meanwhile so I can compare?

Attached the test log

test-failures.txt

Thanks. Unfortunately those are some of the tests which always fail for me.

Here is how they fail:

/Users/mizvekov/Repos/llvm/2/build/rwd/bin/clang++  -std=c++11 -g -O0 -isysroot "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk" -arch arm64  -I/Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make/../../../../..//include -I/Users/mizvekov/Repos/llvm/2/build/rwd/tools/lldb/include -I/Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list -I/Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make -include /Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make/test_common.h  -fno-limit-debug-info    -nostdlib++ -nostdinc++ -cxx-isystem /Users/mizvekov/Repos/llvm/2/build/rwd/include/c++/v1  --driver-mode=g++ -MT main.o -MD -MP -MF main.d -c -o main.o /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
/Users/mizvekov/Repos/llvm/2/build/rwd/bin/clang  main.o -g -O0 -isysroot "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk" -arch arm64  -I/Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make/../../../../..//include -I/Users/mizvekov/Repos/llvm/2/build/rwd/tools/lldb/include -I/Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list -I/Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make -include /Users/mizvekov/Repos/llvm/2/lldb/packages/Python/lldbsuite/test/make/test_common.h  -fno-limit-debug-info     -L/Users/mizvekov/Repos/llvm/2/build/rwd/lib -Wl,-rpath,/Users/mizvekov/Repos/llvm/2/build/rwd/lib -lc++ --driver-mode=g++ -o "a.out"
ld64.lld: error: undefined symbol: std::terminate()
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol __clang_call_terminate+0xc)

ld64.lld: error: undefined symbol: __cxa_begin_catch
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol __clang_call_terminate+0x8)

ld64.lld: error: undefined symbol: operator delete(void*)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol void std::__1::__libcpp_deallocate[abi:se220000]<std::__1::__list_node<int, void*>>(std::__1::__type_identity<std::__1::__list_node<int, void*>>::type*, std::__1::__element_count, unsigned long)+0x2c)

ld64.lld: error: undefined symbol: __cxa_throw
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x30)

ld64.lld: error: undefined symbol: std::bad_array_new_length::~bad_array_new_length()
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x2c)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x28)

ld64.lld: error: undefined symbol: typeinfo for std::bad_array_new_length
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x24)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x20)

ld64.lld: error: undefined symbol: std::bad_array_new_length::bad_array_new_length()
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x18)

ld64.lld: error: undefined symbol: __cxa_allocate_exception
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__throw_bad_array_new_length[abi:se220000]()+0x10)

ld64.lld: error: undefined symbol: operator new(unsigned long)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(symbol std::__1::__list_node<int, void*>* std::__1::__libcpp_allocate[abi:se220000]<std::__1::__list_node<int, void*>>(std::__1::__element_count, unsigned long)+0x28)

ld64.lld: error: undefined symbol: __gxx_personality_v0
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(__compact_unwind+0xb0)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(__compact_unwind+0x2f0)
>>> referenced by /Users/mizvekov/Repos/llvm/2/lldb/test/API/commands/expression/import-std-module/list/main.cpp
>>>               main.o:(__compact_unwind+0x310)
>>> referenced 1 more times

This is even on main branch. I figured this has something to do with the apple libcxx type aware allocator mess, but I wonder if there is a way around that problem I am not aware of.

I managed to fix that, it was some problem with using lld instead of the macOS linker.

Alright, these three macOS-only test failures are fixed now, I tested locally.

I am moving ahead with the merge.